feat: align with MCP 2026-07-28 - #40
Conversation
Update viem, Zod, runtime types and compatible transitive packages. Use TypeScript 5.9 as a development dependency and add the matching v2 client for transport interoperability tests. Builds, type checking and all 38 existing tests pass.
Enforce HTTP Accept and protocol-version headers and return JSON-RPC parser errors. Verify final metadata, encoded names, custom parameter headers and both stdio eras through SDK clients. Propagate CLI startup failures, derive server identity from package metadata and type-check tests. All 53 tests and both Node bundles pass.
Run frozen installs, type checking, both builds and protocol tests across Node 20, 22, 24 and 26. Update release actions and use Node 24; validate before atomically pushing a new release commit and tag without rewriting history. Document the dependency baseline and reproducible verification command. Actionlint and package dry-run pass.
Reject unsupported POST media types before consuming uploads. Route accepted bodies through the bounded Express reader and always supply a parsed value to prevent the SDK raw-stream fallback. Keep parser and encoding failures in JSON-RPC form. Regression tests cover unfinished uploads, chunked and gzip size limits, empty bodies and unsupported encodings; all 56 tests pass.
wayzeek
left a comment
wayzeek
left a comment
There was a problem hiding this comment.
Two independent passes over origin/main...HEAD (7 commits, 36 files), one by hand and one by Codex, each covering both the Standards and Spec axes. The detail is inline.
Every finding was reproduced against the code before it went up. Roughly one in three arrived wrong and those were dropped or corrected in place, including one suggested fix that does not compile.
Nothing blocks the merge. Two are worth a commit before this runs anywhere but localhost, and they are the same mistake twice: a loopback address is read as proof the deployment is unreachable, once for whether OAuth is required (http-server.ts) and once for whether HTTPS is (auth.ts).
Spec conformance I drove against the running server, and it passes: GET and DELETE on the endpoint return 405, an unknown RPC returns 404 with -32601, an invalid Origin returns 403, a missing MCP-Protocol-Version returns 400 with -32020, and the error-code renumbering the upgrade doc claims matches the published changelog.
bun run check is red on macOS for two tests and green on Linux CI; notes on the test file.
| const oauthConfiguration = await loadOAuthResourceServerConfiguration({ | ||
| isLocalHost: isLocalHost(HOST) | ||
| }).catch((error: unknown) => { | ||
| console.error( | ||
| `HTTP authorization configuration error: ${error instanceof Error ? error.message : String(error)}` | ||
| ); | ||
| process.exit(1); |
There was a problem hiding this comment.
Whether OAuth is required is decided from MCP_HOST alone, which stands in for reachability rather than measuring it. Behind a reverse proxy you bind loopback and add the public name to MCP_ALLOWED_HOSTS, and this comes up with oauthConfiguration === undefined.
I ran it: allowedHostnames: ["mcp.example.com"], no OAuth configured, and an unauthenticated tools/call for sign_message returned 200 with a minted confirmation state and no WWW-Authenticate. The only thing between that caller and a wallet signature is the elicitation round trip, which the same caller controls.
Refusing startup when OAuth is off and MCP_ALLOWED_HOSTS names a non-loopback host would close it.
| function requireSecureResourceUrl(value: URL, label: string): void { | ||
| if ( | ||
| value.protocol !== "https:" | ||
| && !(value.protocol === "http:" && isLoopbackHostname(value.hostname)) | ||
| ) { | ||
| throw new Error(`${label} must use HTTPS unless it is a loopback URL`); | ||
| } | ||
| } |
There was a problem hiding this comment.
requireSecureResourceUrl waives HTTPS for any loopback URL regardless of how the process is bound, and isLocalHost never reaches it.
Calling the loader with isLocalHost: false and MCP_PUBLIC_URL=http://localhost:3001/mcp returns a live config whose resourceServerUrl is that localhost URL. That is what protected-resource metadata publishes and what expectedAudience defaults to at line 416, so remote clients get directed at their own loopback and the audience check expects a localhost resource. The upgrade doc and README:284 both say non-local deployments require HTTPS.
Gating the exception on options.isLocalHost closes it. Found by the Codex pass; I reproduced it.
| ); | ||
| } | ||
|
|
||
| const clientId = tokenInfo.client_id ?? tokenInfo.sub; |
There was a problem hiding this comment.
Codex flagged this and recommended requiring client_id outright. I would keep the fallback, noting it here so the decision is on the record.
It does reproduce: a token carrying only sub: "end-user" verifies and lands as AuthInfo.clientId. But RFC 7662 makes client_id optional in the introspection response, so rejecting on its absence fails closed against compliant authorization servers, and nothing in this tree makes an authorization decision from clientId. Only req.auth?.scopes at http-app.ts:178 and the token at request-state.ts:57 are consumed.
It starts to matter when the audit logging under Follow-up Work lands. Recording which claim the identity came from covers that without the interop cost.
| } from "@modelcontextprotocol/server"; | ||
|
|
||
| const CONFIRMATION_STATE_TTL_SECONDS = 5 * 60; | ||
| const requestStateKey = crypto.getRandomValues(new Uint8Array(32)); |
There was a problem hiding this comment.
This key is per process, and consumedConfirmationNonces on the next line is too.
I minted a confirmation in one process and replayed it with an accepted response into a second: refused, and re-prompted with a fresh input_required. So behind more than one replica clients loop on confirmation forever, and single-use replay protection does not span replicas.
It fails closed, so this is deployment guidance rather than a hole. The comment at line 49 covers the restart case; the replica case is the one that bites, because HTTP being stateless in the protocol sense is exactly what invites horizontal scaling. Worth a line next to the rate-limiting and audit-logging items under Follow-up Work.
| jsonrpc: "2.0", | ||
| ...(typeof req.body?.id === "string" || typeof req.body?.id === "number" | ||
| ? { id: req.body.id } : {}), | ||
| error: { code: -32020, message: "Missing MCP-Protocol-Version header" } |
There was a problem hiding this comment.
-32020 is right. I checked the spec rather than guessing: Server Validation lists a missing required standard header (MCP-Protocol-Version, Mcp-Method, Mcp-Name) as a HeaderMismatch condition returning 400, so this branch matches.
It reads as a magic number beside the ProtocolErrorCode.InvalidRequest branches above and below, and the independent pass flagged it too, suggesting ProtocolErrorCode.HeaderMismatch. That does not exist: the enum carries ParseError, InvalidRequest, MethodNotFound, InvalidParams, InternalError, ResourceNotFound, MissingRequiredClientCapability, UnsupportedProtocolVersion and UrlElicitationRequired, and tsc rejects the member. A named constant in protocol.ts beside the other protocol constants is the fix that compiles.
| ## Final-Spec Differences from the RC | ||
|
|
||
| The repository no longer carries RC behavior for the following changes: |
There was a problem hiding this comment.
This section describes behavior the repo no longer has. The opening paragraph already says the RC adapter is gone, so a reader arriving now has no RC to compare it against.
| } | ||
| }); | ||
|
|
||
| test("rejects unsupported media types before waiting for the request body", async () => { |
There was a problem hiding this comment.
This test and "limits chunked and decompressed JSON bodies" below both fail on macOS, on the locally installed Bun 1.2.13 and on the 1.4.2 that CI pins.
The assertions are right. I drove the same two scenarios against the app under Node 24 and it behaves exactly as asserted, returning an early 415 before the upload finishes and 413 with the JSON-RPC body. It is Bun's node:http shim on darwin, and CI is green on 67e11cf.
Nothing to change here. It does mean bun run check is red for macOS contributors with nothing saying why, so a line in the doc's Verification section would save someone an afternoon.
| const confirmation = await requireConfirmation( | ||
| ctx, | ||
| "sign_message", | ||
| { message }, | ||
| `Sign this message with the configured wallet?\n\n${message}` | ||
| ); | ||
| if (confirmation) { | ||
| return confirmation; | ||
| } |
There was a problem hiding this comment.
Confirmation sits outside the try here, where the other five wallet-backed handlers call it inside theirs (transfer_native at 1319-1322 is the nearest comparison). Raised by the independent pass.
I went looking for a divergence and could not produce one: a forged requestState against sign_message and against transfer_native both come back as a fresh input_required, because the SDK's verify returns empty rather than throwing. So this is consistency rather than a live bug today. It is still the one handler where a future throwing path in the helper would escape the isError: true shape the rest of the file guarantees.
| return inputRequired({ | ||
| inputRequests: { | ||
| confirmation: inputRequired.elicit({ | ||
| message, | ||
| requestedSchema: confirmationSchema | ||
| }) | ||
| }, | ||
| requestState: await mintConfirmationRequestState(operationDigest, ctx) | ||
| }); |
There was a problem hiding this comment.
This envelope repeats lines 274-282 verbatim, so a change to the confirmation shape needs both edits. Flagged by the independent pass as possible Duplicated Code and I agree, though it is small enough to be a judgement call rather than something to insist on.
| async function main() { | ||
| try { | ||
| const server = await startServer(); | ||
| const transport = new StdioServerTransport(); | ||
| await server.connect(transport); | ||
| console.error("EVM MCP Server running on stdio"); | ||
| await runStdioServer(); |
There was a problem hiding this comment.
runStdioServer is declared (): StdioServerHandle and returns synchronously, so the await is decorative. Harmless, since a synchronous throw is still caught by the surrounding try, but it reads as though startup were asynchronous. Raised by the independent pass; I verified the signature and that the error boundary is unaffected.
2 participants
What
2026-07-28protocol and TypeScript SDK v2 packagesWhy
The repository still targeted the pre-release/legacy MCP surface after the final
2026-07-28standard and SDK v2 packages shipped. That left the transport lifecycle, tool contracts, wallet confirmation flow, and HTTP authorization model out of sync with the released wire protocol.Impact
structuredContentRoot cause
Protocol-specific behavior was spread across a local JSON-RPC adapter, hand-rolled HTTP transport logic, older SDK assumptions, and duplicated service helpers. The final standard changed lifecycle negotiation, per-request metadata, headers, error codes, MRTR behavior, caching, and server identity placement, so updating individual call sites was insufficient.
Checks
bun run test:mcp— 38 tests, 191 assertionsbunx tsc --noEmitbun run buildbun run build:httpgit diff --checkget_supported_networksTooling caveat
Inspector v2.0.0 and conformance alpha.10 were published too recently for the workspace's seven-day package-age safeguard. They were not bypassed. The remaining alpha.9 results are either pre-final expectations (
clientInforequired andserverInfoin the discover body) or require synthetic conformance-only fixture tools this production server intentionally does not expose.